Fix max-allowable-numa-nodes documentation#54271
Fix max-allowable-numa-nodes documentation#54271komuta wants to merge 1 commit intokubernetes:mainfrom
Conversation
The current documentation implies that the value for `max-allowable-numa-nodes` is a boolean, whereas an integer is actually expected. Adding also a kubelet config example since TopologyManager options are wrapped as string. See: https://github.com/kubernetes/kubernetes/blob/bb52ae5e24a4eb118460e8feadee86b9f41b4244/pkg/kubelet/cm/topologymanager/policy_options.go#L61
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @komuta! |
k8s-ci-robot
added
size/S
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hello @komuta Thanks for your contribution, kindly sign the CLA to enable your contribution to be reviewed |
|
Thanks @Caesarsage, I'll get the CLA signed ASAP (I'm dependent of office hours in CET 😅 ). |
|
/easycla |
k8s-ci-robot
added
cncf-cla: yes
NIRANJAN0125
left a comment
NIRANJAN0125
left a comment
There was a problem hiding this comment.
Thanks for working on fix max-allowable-numa-nodes documentation. Mostly looks good, a few suggestions.
|
|
||
| ```yaml | ||
| topologyManagerPolicyOptions: | ||
| max-allowable-numa-nodes: "32" |
There was a problem hiding this comment.
max-allowable-numa-nodes=<numa_nodes_threshold>, where numa_nodes_threshold is an integer — The old text said max-allowable-numa-nodes=true (boolean). This PR changes it to accept an integer. This is a significant behavioral claim — please link to the upstream KEP or source code commit that changed this from a boolean to an integer threshold.
|
|
||
| ```yaml | ||
| topologyManagerPolicyOptions: | ||
| max-allowable-numa-nodes: "32" |
There was a problem hiding this comment.
If the value is an integer threshold, what does "32" mean exactly? Does it raise the default limit from 8 to 32? The text says "defining the new NUMA nodes threshold for starting the kubelet" but the existing paragraph at L276 says "Setting a value of max-allowable-numa-nodes does not affect the latency of pod admission". These two sentences are now contradictory in tone. Clarify: does setting "32" mean "allow kubelet to start with Topology Manager on nodes with up to 32 NUMA nodes"?
L267–L272 The note still says "nodes with more than 8 NUMA
| more than 8 NUMA nodes are detected. | ||
|
|
||
| {{< note >}} | ||
| If you select the `max-allowable-numa-nodes` policy option, nodes with more than 8 NUMA nodes can |
There was a problem hiding this comment.
Should as be updated as well ? As the note still says "nodes with more than 8 NUMA nodes can be allowed" — but with the new integer syntax, this note should be updated to reflect that the threshold is now configurable, not just "more than 8". The note and the example are inconsistent.
5 participants
The current documentation implies that the value for
max-allowable-numa-nodesis a boolean, whereas an integer is actually expected. Adding also a kubelet config example since TopologyManager options are wrapped as string.See: https://github.com/kubernetes/kubernetes/blob/bb52ae5e24a4eb118460e8feadee86b9f41b4244/pkg/kubelet/cm/topologymanager/policy_options.go#L61